-
-
Notifications
You must be signed in to change notification settings - Fork 331
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
deps: Move from winapi
to windows
#950
deps: Move from winapi
to windows
#950
Conversation
da287bc
to
88c1d32
Compare
Currently blocked by microsoft/windows-rs#2370 because Also, it should be noted There is microsoft/windows-rs#2358 (comment) which could probably help, and it's now possible to build crates above |
Can you point to what's blocking you?
|
I'd rather wait for the alias to be available in the dependency rather than adding it in Overall, it seems to go into the right direction, but |
@GuillaumeGomez Don't think we have any plans to add that alias. It's not needed as the PR demonstrates? |
If it's used by the windows API, then it's needed, even if it's just an alias. Just like |
88c1d32
to
724eca5
Compare
Ah, that's sad. |
We can update the minimum required version. It'll just require us to change sysinfo major version number. |
What do you prefer ? Doing it in this MR (in new commits) or in a separate one ? |
In this one, otherwise CI will always fail. Just put it into its own commit. |
b9b326b
to
6bd6bcf
Compare
The latest version of the https://kennykerr.ca/rust-getting-started/windows-or-windows-sys.html |
6bd6bcf
to
fa64a9e
Compare
fa64a9e
to
62074a1
Compare
048d52c
to
b7c1a0a
Compare
57a7156
to
006bb9a
Compare
I'm not managing to reproduce locally, makes searching for the bug much more annoying than it should be |
Try to update CI script for windows to run with |
006bb9a
to
935c976
Compare
Rebased and green CI, should be good to go 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from the few code style nits, it looks great, thanks for working on this!
`winapi` is in maintenance mode and the new blessed way to access Windows APIs are the `windows` and `windows-sys` crates. I was forced to use `windows` because `sysinfo` uses the COM APIs which are not present in `windows-sys`.
935c976
to
5a9b8d6
Compare
All fixed :) |
Looks all good to me, thanks a lot! |
winapi
is in maintenance mode and the new blessed way to access Windows APIs are thewindows
and
windows-sys
crates. I was forced to usewindows
becausesysinfo
uses the COM APIswhich are not present in
windows-sys
.